Skip to content

[Fast-Reboot]: fast-reboot-dump.py script connects to the DB by TCP#225

Merged
lguohan merged 4 commits intosonic-net:masterfrom
pavel-shirshov:pavelsh/as_root
Jul 15, 2018
Merged

[Fast-Reboot]: fast-reboot-dump.py script connects to the DB by TCP#225
lguohan merged 4 commits intosonic-net:masterfrom
pavel-shirshov:pavelsh/as_root

Conversation

@pavel-shirshov
Copy link
Contributor

@pavel-shirshov pavel-shirshov commented Mar 29, 2018

- What I did
Change connection method to DB for fast-reboot-dump.py script. Now it's TCP, previously it was UNIX socket which caused issues in case non-root user try to run the script.

- How I did it
Add TCP arguments for Sonic DB connector

- How to verify it
Try to run fast-reboot-dump.py as a regular user

- Previous command output (if the output of a command-line utility has changed)

- New command output (if the output of a command-line utility has changed)

-->

def main():
if os.geteuid() != 0:
print >> sys.stderr, 'Please run as root'
sys.exit(0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should exit 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Copy link
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change is good; we should warn the user and exit if not root. However, what I was referring to yesterday was line 244, which, if run by a non-root user will quietly hang forever.

Even though this script is meant to be always run by superuser, I think you might want to change line 244 to db = swsssdk.SonicV2Connector(host='127.0.0.1') or add a comment that the current call (with no parameter) will hang forever if not run by superuser, just in case someone models a new script after this one, they will have a better understanding of the behavior.

FYI, we will be deprecating swsssdk in the near future (replacing with the Python binding of swss-common, in which case this will no longer be an issue.

@pavel-shirshov pavel-shirshov changed the title [Fast-Reboot]: Check user id for fast-reboot-dump.py script [Fast-Reboot]: fast-reboot-dump.py script connects to the DB by TCP Mar 29, 2018
@pavel-shirshov
Copy link
Contributor Author

Fixed db connection string In three places.

@lguohan lguohan merged commit f1017ab into sonic-net:master Jul 15, 2018
mihirpat1 pushed a commit to mihirpat1/sonic-utilities that referenced this pull request Sep 15, 2023
kktheballer pushed a commit to kktheballer/sonic-utilities that referenced this pull request Jan 14, 2026
```<br>* 1b02df0 - (HEAD -> 202506) Merge branch '202505' of https://github.com/sonic-net/sonic-utilities into 202506 (2025-08-09) [Sonic Automation]
* 6354429 - (origin/202505) Revert "[SPM] Rename the variable tag to docker-image-reference" (sonic-net#4026) (2025-08-09) [mssonicbld]<br>```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants